-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][flang][mlir] Support -frecord-command-line option #102975
[clang][flang][mlir] Support -frecord-command-line option #102975
Conversation
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-mlir Author: Tarun Prabhu (tarunprabhu) ChangesAdd support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang. Notes for reviewers: In #98517, the This required a change to If others have better suggestions for how to handle this, I am happy to make changes. Patch is 28.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102975.diff 24 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0b38139bd27972..670e3658b6e85c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an
argument are escaped with backslashes. This format differs from the format of
the equivalent section produced by GCC with the -frecord-gcc-switches flag.
This option is currently only supported on ELF targets.}]>,
- Group<f_clang_Group>;
+ Group<f_clang_Group>,
+ Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
def fno_record_command_line : Flag<["-"], "fno-record-command-line">,
- Group<f_clang_Group>;
+ Group<f_clang_Group>,
+ Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;
def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;
def fcommon : Flag<["-"], "fcommon">, Group<f_Group>,
@@ -7074,6 +7076,9 @@ def mrelocation_model : Separate<["-"], "mrelocation-model">,
NormalizedValues<["Static", "PIC_", "ROPI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
MarshallingInfoEnum<CodeGenOpts<"RelocationModel">, "PIC_">;
def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
+def record_command_line : Separate<["-"], "record-command-line">,
+ HelpText<"The string to embed in the .LLVM.command.line section.">,
+ MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
} // let Visibility = [CC1Option, CC1AsOption, FC1Option]
@@ -7094,9 +7099,6 @@ def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">,
def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
HelpText<"The string to embed in the Dwarf debug flags record.">,
MarshallingInfoString<CodeGenOpts<"DwarfDebugFlags">>;
-def record_command_line : Separate<["-"], "record-command-line">,
- HelpText<"The string to embed in the .LLVM.command.line section.">,
- MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
def compress_debug_sections_EQ : Joined<["-", "--"], "compress-debug-sections=">,
HelpText<"DWARF debug sections compression type">, Values<"none,zlib,zstd">,
NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Zlib", "Zstd"]>,
diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 32a4378ab499fa..0b92077384e9bc 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -48,6 +48,7 @@ add_clang_library(clangDriver
ToolChains/BareMetal.cpp
ToolChains/Clang.cpp
ToolChains/CommonArgs.cpp
+ ToolChains/CommonUtils.cpp
ToolChains/CrossWindows.cpp
ToolChains/CSKYToolChain.cpp
ToolChains/Cuda.cpp
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c698d38b80e578..e98093c855548d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -21,6 +21,7 @@
#include "Arch/VE.h"
#include "Arch/X86.h"
#include "CommonArgs.h"
+#include "CommonUtils.h"
#include "Hexagon.h"
#include "MSP430.h"
#include "PS4CPU.h"
@@ -94,24 +95,6 @@ static void CheckCodeGenerationOptions(const Driver &D, const ArgList &Args) {
<< "-static";
}
-// Add backslashes to escape spaces and other backslashes.
-// This is used for the space-separated argument list specified with
-// the -dwarf-debug-flags option.
-static void EscapeSpacesAndBackslashes(const char *Arg,
- SmallVectorImpl<char> &Res) {
- for (; *Arg; ++Arg) {
- switch (*Arg) {
- default:
- break;
- case ' ':
- case '\\':
- Res.push_back('\\');
- break;
- }
- Res.push_back(*Arg);
- }
-}
-
/// Apply \a Work on the current tool chain \a RegularToolChain and any other
/// offloading tool chain that is associated with the current action \a JA.
static void
@@ -7662,31 +7645,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// Also record command line arguments into the debug info if
// -grecord-gcc-switches options is set on.
// By default, -gno-record-gcc-switches is set on and no recording.
- auto GRecordSwitches =
- Args.hasFlag(options::OPT_grecord_command_line,
- options::OPT_gno_record_command_line, false);
- auto FRecordSwitches =
- Args.hasFlag(options::OPT_frecord_command_line,
- options::OPT_fno_record_command_line, false);
- if (FRecordSwitches && !Triple.isOSBinFormatELF() &&
- !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
- D.Diag(diag::err_drv_unsupported_opt_for_target)
- << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
- << TripleStr;
- if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
- ArgStringList OriginalArgs;
- for (const auto &Arg : Args)
- Arg->render(Args, OriginalArgs);
-
- SmallString<256> Flags;
- EscapeSpacesAndBackslashes(Exec, Flags);
- for (const char *OriginalArg : OriginalArgs) {
- SmallString<128> EscapedArg;
- EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
- Flags += " ";
- Flags += EscapedArg;
- }
- auto FlagsArgString = Args.MakeArgString(Flags);
+ auto GRecordSwitches = false;
+ auto FRecordSwitches = false;
+ if (ShouldRecordCommandLine(TC, Args, FRecordSwitches, GRecordSwitches)) {
+ auto FlagsArgString = RenderEscapedCommandLine(TC, Args);
if (TC.UseDwarfDebugFlags() || GRecordSwitches) {
CmdArgs.push_back("-dwarf-debug-flags");
CmdArgs.push_back(FlagsArgString);
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.cpp b/clang/lib/Driver/ToolChains/CommonUtils.cpp
new file mode 100644
index 00000000000000..20d76990517a43
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.cpp
@@ -0,0 +1,76 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CommonUtils.h"
+#include "clang/Driver/Driver.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::driver;
+
+namespace clang {
+
+void EscapeSpacesAndBackslashes(const char *Arg,
+ llvm::SmallVectorImpl<char> &Res) {
+ for (; *Arg; ++Arg) {
+ switch (*Arg) {
+ default:
+ break;
+ case ' ':
+ case '\\':
+ Res.push_back('\\');
+ break;
+ }
+ Res.push_back(*Arg);
+ }
+}
+
+const char *RenderEscapedCommandLine(const ToolChain &TC,
+ const llvm::opt::ArgList &Args) {
+ const Driver &D = TC.getDriver();
+ const char *Exec = D.getClangProgramPath();
+
+ llvm::opt::ArgStringList OriginalArgs;
+ for (const auto &Arg : Args)
+ Arg->render(Args, OriginalArgs);
+
+ llvm::SmallString<256> Flags;
+ EscapeSpacesAndBackslashes(Exec, Flags);
+ for (const char *OriginalArg : OriginalArgs) {
+ llvm::SmallString<128> EscapedArg;
+ EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
+ Flags += " ";
+ Flags += EscapedArg;
+ }
+
+ return Args.MakeArgString(Flags);
+}
+
+bool ShouldRecordCommandLine(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ bool &FRecordCommandLine,
+ bool &GRecordCommandLine) {
+ const Driver &D = TC.getDriver();
+ const llvm::Triple &Triple = TC.getEffectiveTriple();
+ const std::string &TripleStr = Triple.getTriple();
+
+ FRecordCommandLine =
+ Args.hasFlag(options::OPT_frecord_command_line,
+ options::OPT_fno_record_command_line, false);
+ GRecordCommandLine =
+ Args.hasFlag(options::OPT_grecord_command_line,
+ options::OPT_gno_record_command_line, false);
+ if (FRecordCommandLine && !Triple.isOSBinFormatELF() &&
+ !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
+ D.Diag(diag::err_drv_unsupported_opt_for_target)
+ << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
+ << TripleStr;
+
+ return FRecordCommandLine || TC.UseDwarfDebugFlags() || GRecordCommandLine;
+}
+
+} // namespace clang
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.h b/clang/lib/Driver/ToolChains/CommonUtils.h
new file mode 100644
index 00000000000000..1ae1c54cf58db4
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.h
@@ -0,0 +1,44 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+
+#include "clang/Driver/ToolChain.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
+
+namespace clang {
+
+// Add backslashes to escape spaces and other backslashes.
+// This is used for the space-separated argument list specified with
+// the -dwarf-debug-flags option.
+void EscapeSpacesAndBackslashes(const char *Arg,
+ llvm::SmallVectorImpl<char> &Res);
+
+/// Join the args in the given ArgList, escape spaces and backslashes and
+/// return the joined string. This is used when saving the command line as a
+/// result of using either the -frecord-command-line or -grecord-command-line
+/// options. The lifetime of the returned c-string will match that of the Args
+/// argument.
+const char *RenderEscapedCommandLine(const clang::driver::ToolChain &TC,
+ const llvm::opt::ArgList &Args);
+
+/// Check if the command line should be recorded in the object file. This is
+/// done if either -frecord-command-line or -grecord-command-line options have
+/// been passed. This also does some error checking since -frecord-command-line
+/// is currently only supported on ELF platforms. The last two boolean
+/// arguments are out parameters and will be set depending on the command
+/// line options that were passed.
+bool ShouldRecordCommandLine(const clang::driver::ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ bool &FRecordCommandLine,
+ bool &GRecordCommandLine);
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6ce79d27e98c48..2ef14e306cf324 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -9,6 +9,7 @@
#include "Flang.h"
#include "Arch/RISCV.h"
#include "CommonArgs.h"
+#include "CommonUtils.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Driver/Options.h"
@@ -885,6 +886,20 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
addDashXForInput(Args, Input, CmdArgs);
+ bool FRecordCmdLine = false;
+ bool GRecordCmdLine = false;
+ if (ShouldRecordCommandLine(TC, Args, FRecordCmdLine, GRecordCmdLine)) {
+ const char *CmdLine = RenderEscapedCommandLine(TC, Args);
+ if (FRecordCmdLine) {
+ CmdArgs.push_back("-record-command-line");
+ CmdArgs.push_back(CmdLine);
+ }
+ if (TC.UseDwarfDebugFlags() || GRecordCmdLine) {
+ CmdArgs.push_back("-dwarf-debug-flags");
+ CmdArgs.push_back(CmdLine);
+ }
+ }
+
CmdArgs.push_back(Input.getFilename());
// TODO: Replace flang-new with flang once the new driver replaces the
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
index ac7fcbcba4f747..f19943335737b9 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.h
+++ b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -63,6 +63,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// The directory where temp files are stored if specified by -save-temps
std::optional<std::string> SaveTempsDir;
+ /// The string containing the commandline for the llvm.commandline metadata.
+ std::optional<std::string> RecordCommandLine;
+
/// The name of the file to which the backend should save YAML optimization
/// records.
std::string OptRecordFile;
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index 4379ed512cdf0a..8ea5ed52e28218 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -14,6 +14,8 @@
#define FORTRAN_LOWER_BRIDGE_H
#include "flang/Common/Fortran.h"
+#include "flang/Frontend/CodeGenOptions.h"
+#include "flang/Frontend/TargetOptions.h"
#include "flang/Lower/AbstractConverter.h"
#include "flang/Lower/EnvironmentDefault.h"
#include "flang/Lower/LoweringOptions.h"
@@ -65,11 +67,13 @@ class LoweringBridge {
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, llvm::StringRef tuneCPU) {
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
+ const Fortran::frontend::CodeGenOptions &codeGenOptions) {
return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
targetCharacteristics, allCooked, triple, kindMap,
loweringOptions, envDefaults, languageFeatures,
- targetMachine, tuneCPU);
+ targetMachine, targetOptions, codeGenOptions);
}
//===--------------------------------------------------------------------===//
@@ -148,7 +152,9 @@ class LoweringBridge {
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU);
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
+ const Fortran::frontend::CodeGenOptions &codeGenOptions);
LoweringBridge() = delete;
LoweringBridge(const LoweringBridge &) = delete;
diff --git a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
index e45011c8e02a33..2df14f83c11e17 100644
--- a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
+++ b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
@@ -77,6 +77,12 @@ void setIdent(mlir::ModuleOp mod, llvm::StringRef ident);
/// Get the compiler identifier from the Module.
llvm::StringRef getIdent(mlir::ModuleOp mod);
+/// Set the command line used in this invocation.
+void setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine);
+
+/// Get the command line used in this invocation.
+llvm::StringRef getCommandline(mlir::ModuleOp mod);
+
/// Helper for determining the target from the host, etc. Tools may use this
/// function to provide a consistent interpretation of the `--target=<string>`
/// command-line option.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..4e8fd44f526a5c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -348,6 +348,12 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
opts.SaveTempsDir = a->getValue();
+ // -record-command-line option.
+ if (const llvm::opt::Arg *a =
+ args.getLastArg(clang::driver::options::OPT_record_command_line)) {
+ opts.RecordCommandLine = a->getValue();
+ }
+
// -mlink-builtin-bitcode
for (auto *a :
args.filtered(clang::driver::options::OPT_mlink_builtin_bitcode))
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 5c86bd947ce73f..909e5909938d87 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -298,7 +298,7 @@ bool CodeGenAction::beginSourceFileAction() {
kindMap, ci.getInvocation().getLoweringOpts(),
ci.getInvocation().getFrontendOpts().envDefaults,
ci.getInvocation().getFrontendOpts().features, targetMachine,
- ci.getInvocation().getTargetOpts().cpuToTuneFor);
+ ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
// Fetch module from lb, so we can set
mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ccbb481f472d81..2b42c3e355fbcc 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -6065,7 +6065,9 @@ Fortran::lower::LoweringBridge::LoweringBridge(
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU)
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOpts,
+ const Fortran::frontend::CodeGenOptions &cgOpts)
: semanticsContext{semanticsContext}, defaultKinds{defaultKinds},
intrinsics{intrinsics}, targetCharacteristics{targetCharacteristics},
cooked{&cooked}, context{context}, kindMap{kindMap},
@@ -6122,11 +6124,13 @@ Fortran::lower::LoweringBridge::LoweringBridge(
fir::setTargetTriple(*module.get(), triple);
fir::setKindMapping(*module.get(), kindMap);
fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
- fir::setTuneCPU(*module.get(), tuneCPU);
+ fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
fir::support::setMLIRDataLayout(*module.get(),
targetMachine.createDataLayout());
fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
+ if (cgOpts.RecordCommandLine)
+ fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
}
void Fortran::lower::genCleanUpInRegionIfAny(
diff --git a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
index 5bd8e2d4336361..01c0be66d1ecc3 100644
--- a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
+++ b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
@@ -130,6 +130,22 @@ llvm::StringRef fir::getIdent(mlir::ModuleOp mod) {
return {};
}
+void fir::setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine) {
+ if (cmdLine.empty())
+ return;
+
+ mlir::MLIRContext *ctx = mod.getContext();
+ mod->setAttr(mlir::LLVM::LLVMDialect::getCommandlineAttrName(),
+ mlir::StringAttr::get(ctx, cmdLine));
+}
+
+llvm::StringRef fir::getCommandline(mlir::ModuleOp mod) {
+ if (auto attr = mod->getAttrOfType<mlir::StringAttr>(
+ mlir::LLVM::LLVMDialect::getCommandlineAttrName()))
+ return attr;
+ return {};
+}
+
std::string fir::determineTargetTriple(llvm::StringRef triple) {
// Treat "" or "default" as stand-ins for the default machine.
if (triple.empty() || triple == "default")
diff --git a/flang/test/Driver/frecord-command-line.f90 b/flang/test/Driver/frecord-command-line.f90
new file mode 100644
index 00000000000000..3dc65587ee2563
--- /dev/null
+++ b/flang/test/Driver/frecord-command-line.f90
@@ -0,0 +1,11 @@
+! This only checks that the command line is correctly passed on to the
+! -record-command-line option FC1 option and that the latter does not complain
+! about anything. The correct lowering to a module attribute and beyond will
+! be checked in other tests.
+!
+! RUN: %flang -### -frecord-command-line %s 2>&1 | FileCheck --check-prefix=CHECK-RECORD ...
[truncated]
|
@llvm/pr-subscribers-clang Author: Tarun Prabhu (tarunprabhu) ChangesAdd support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang. Notes for reviewers: In #98517, the This required a change to If others have better suggestions for how to handle this, I am happy to make changes. Patch is 28.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102975.diff 24 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0b38139bd27972..670e3658b6e85c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an
argument are escaped with backslashes. This format differs from the format of
the equivalent section produced by GCC with the -frecord-gcc-switches flag.
This option is currently only supported on ELF targets.}]>,
- Group<f_clang_Group>;
+ Group<f_clang_Group>,
+ Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
def fno_record_command_line : Flag<["-"], "fno-record-command-line">,
- Group<f_clang_Group>;
+ Group<f_clang_Group>,
+ Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;
def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;
def fcommon : Flag<["-"], "fcommon">, Group<f_Group>,
@@ -7074,6 +7076,9 @@ def mrelocation_model : Separate<["-"], "mrelocation-model">,
NormalizedValues<["Static", "PIC_", "ROPI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
MarshallingInfoEnum<CodeGenOpts<"RelocationModel">, "PIC_">;
def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
+def record_command_line : Separate<["-"], "record-command-line">,
+ HelpText<"The string to embed in the .LLVM.command.line section.">,
+ MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
} // let Visibility = [CC1Option, CC1AsOption, FC1Option]
@@ -7094,9 +7099,6 @@ def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">,
def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
HelpText<"The string to embed in the Dwarf debug flags record.">,
MarshallingInfoString<CodeGenOpts<"DwarfDebugFlags">>;
-def record_command_line : Separate<["-"], "record-command-line">,
- HelpText<"The string to embed in the .LLVM.command.line section.">,
- MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
def compress_debug_sections_EQ : Joined<["-", "--"], "compress-debug-sections=">,
HelpText<"DWARF debug sections compression type">, Values<"none,zlib,zstd">,
NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Zlib", "Zstd"]>,
diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 32a4378ab499fa..0b92077384e9bc 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -48,6 +48,7 @@ add_clang_library(clangDriver
ToolChains/BareMetal.cpp
ToolChains/Clang.cpp
ToolChains/CommonArgs.cpp
+ ToolChains/CommonUtils.cpp
ToolChains/CrossWindows.cpp
ToolChains/CSKYToolChain.cpp
ToolChains/Cuda.cpp
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c698d38b80e578..e98093c855548d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -21,6 +21,7 @@
#include "Arch/VE.h"
#include "Arch/X86.h"
#include "CommonArgs.h"
+#include "CommonUtils.h"
#include "Hexagon.h"
#include "MSP430.h"
#include "PS4CPU.h"
@@ -94,24 +95,6 @@ static void CheckCodeGenerationOptions(const Driver &D, const ArgList &Args) {
<< "-static";
}
-// Add backslashes to escape spaces and other backslashes.
-// This is used for the space-separated argument list specified with
-// the -dwarf-debug-flags option.
-static void EscapeSpacesAndBackslashes(const char *Arg,
- SmallVectorImpl<char> &Res) {
- for (; *Arg; ++Arg) {
- switch (*Arg) {
- default:
- break;
- case ' ':
- case '\\':
- Res.push_back('\\');
- break;
- }
- Res.push_back(*Arg);
- }
-}
-
/// Apply \a Work on the current tool chain \a RegularToolChain and any other
/// offloading tool chain that is associated with the current action \a JA.
static void
@@ -7662,31 +7645,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// Also record command line arguments into the debug info if
// -grecord-gcc-switches options is set on.
// By default, -gno-record-gcc-switches is set on and no recording.
- auto GRecordSwitches =
- Args.hasFlag(options::OPT_grecord_command_line,
- options::OPT_gno_record_command_line, false);
- auto FRecordSwitches =
- Args.hasFlag(options::OPT_frecord_command_line,
- options::OPT_fno_record_command_line, false);
- if (FRecordSwitches && !Triple.isOSBinFormatELF() &&
- !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
- D.Diag(diag::err_drv_unsupported_opt_for_target)
- << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
- << TripleStr;
- if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
- ArgStringList OriginalArgs;
- for (const auto &Arg : Args)
- Arg->render(Args, OriginalArgs);
-
- SmallString<256> Flags;
- EscapeSpacesAndBackslashes(Exec, Flags);
- for (const char *OriginalArg : OriginalArgs) {
- SmallString<128> EscapedArg;
- EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
- Flags += " ";
- Flags += EscapedArg;
- }
- auto FlagsArgString = Args.MakeArgString(Flags);
+ auto GRecordSwitches = false;
+ auto FRecordSwitches = false;
+ if (ShouldRecordCommandLine(TC, Args, FRecordSwitches, GRecordSwitches)) {
+ auto FlagsArgString = RenderEscapedCommandLine(TC, Args);
if (TC.UseDwarfDebugFlags() || GRecordSwitches) {
CmdArgs.push_back("-dwarf-debug-flags");
CmdArgs.push_back(FlagsArgString);
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.cpp b/clang/lib/Driver/ToolChains/CommonUtils.cpp
new file mode 100644
index 00000000000000..20d76990517a43
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.cpp
@@ -0,0 +1,76 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CommonUtils.h"
+#include "clang/Driver/Driver.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::driver;
+
+namespace clang {
+
+void EscapeSpacesAndBackslashes(const char *Arg,
+ llvm::SmallVectorImpl<char> &Res) {
+ for (; *Arg; ++Arg) {
+ switch (*Arg) {
+ default:
+ break;
+ case ' ':
+ case '\\':
+ Res.push_back('\\');
+ break;
+ }
+ Res.push_back(*Arg);
+ }
+}
+
+const char *RenderEscapedCommandLine(const ToolChain &TC,
+ const llvm::opt::ArgList &Args) {
+ const Driver &D = TC.getDriver();
+ const char *Exec = D.getClangProgramPath();
+
+ llvm::opt::ArgStringList OriginalArgs;
+ for (const auto &Arg : Args)
+ Arg->render(Args, OriginalArgs);
+
+ llvm::SmallString<256> Flags;
+ EscapeSpacesAndBackslashes(Exec, Flags);
+ for (const char *OriginalArg : OriginalArgs) {
+ llvm::SmallString<128> EscapedArg;
+ EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
+ Flags += " ";
+ Flags += EscapedArg;
+ }
+
+ return Args.MakeArgString(Flags);
+}
+
+bool ShouldRecordCommandLine(const ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ bool &FRecordCommandLine,
+ bool &GRecordCommandLine) {
+ const Driver &D = TC.getDriver();
+ const llvm::Triple &Triple = TC.getEffectiveTriple();
+ const std::string &TripleStr = Triple.getTriple();
+
+ FRecordCommandLine =
+ Args.hasFlag(options::OPT_frecord_command_line,
+ options::OPT_fno_record_command_line, false);
+ GRecordCommandLine =
+ Args.hasFlag(options::OPT_grecord_command_line,
+ options::OPT_gno_record_command_line, false);
+ if (FRecordCommandLine && !Triple.isOSBinFormatELF() &&
+ !Triple.isOSBinFormatXCOFF() && !Triple.isOSBinFormatMachO())
+ D.Diag(diag::err_drv_unsupported_opt_for_target)
+ << Args.getLastArg(options::OPT_frecord_command_line)->getAsString(Args)
+ << TripleStr;
+
+ return FRecordCommandLine || TC.UseDwarfDebugFlags() || GRecordCommandLine;
+}
+
+} // namespace clang
diff --git a/clang/lib/Driver/ToolChains/CommonUtils.h b/clang/lib/Driver/ToolChains/CommonUtils.h
new file mode 100644
index 00000000000000..1ae1c54cf58db4
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/CommonUtils.h
@@ -0,0 +1,44 @@
+//===--- CommonUtils.h - Common utilities for the toolchains ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
+
+#include "clang/Driver/ToolChain.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
+
+namespace clang {
+
+// Add backslashes to escape spaces and other backslashes.
+// This is used for the space-separated argument list specified with
+// the -dwarf-debug-flags option.
+void EscapeSpacesAndBackslashes(const char *Arg,
+ llvm::SmallVectorImpl<char> &Res);
+
+/// Join the args in the given ArgList, escape spaces and backslashes and
+/// return the joined string. This is used when saving the command line as a
+/// result of using either the -frecord-command-line or -grecord-command-line
+/// options. The lifetime of the returned c-string will match that of the Args
+/// argument.
+const char *RenderEscapedCommandLine(const clang::driver::ToolChain &TC,
+ const llvm::opt::ArgList &Args);
+
+/// Check if the command line should be recorded in the object file. This is
+/// done if either -frecord-command-line or -grecord-command-line options have
+/// been passed. This also does some error checking since -frecord-command-line
+/// is currently only supported on ELF platforms. The last two boolean
+/// arguments are out parameters and will be set depending on the command
+/// line options that were passed.
+bool ShouldRecordCommandLine(const clang::driver::ToolChain &TC,
+ const llvm::opt::ArgList &Args,
+ bool &FRecordCommandLine,
+ bool &GRecordCommandLine);
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONUTILS_H
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6ce79d27e98c48..2ef14e306cf324 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -9,6 +9,7 @@
#include "Flang.h"
#include "Arch/RISCV.h"
#include "CommonArgs.h"
+#include "CommonUtils.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Driver/Options.h"
@@ -885,6 +886,20 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
addDashXForInput(Args, Input, CmdArgs);
+ bool FRecordCmdLine = false;
+ bool GRecordCmdLine = false;
+ if (ShouldRecordCommandLine(TC, Args, FRecordCmdLine, GRecordCmdLine)) {
+ const char *CmdLine = RenderEscapedCommandLine(TC, Args);
+ if (FRecordCmdLine) {
+ CmdArgs.push_back("-record-command-line");
+ CmdArgs.push_back(CmdLine);
+ }
+ if (TC.UseDwarfDebugFlags() || GRecordCmdLine) {
+ CmdArgs.push_back("-dwarf-debug-flags");
+ CmdArgs.push_back(CmdLine);
+ }
+ }
+
CmdArgs.push_back(Input.getFilename());
// TODO: Replace flang-new with flang once the new driver replaces the
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
index ac7fcbcba4f747..f19943335737b9 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.h
+++ b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -63,6 +63,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// The directory where temp files are stored if specified by -save-temps
std::optional<std::string> SaveTempsDir;
+ /// The string containing the commandline for the llvm.commandline metadata.
+ std::optional<std::string> RecordCommandLine;
+
/// The name of the file to which the backend should save YAML optimization
/// records.
std::string OptRecordFile;
diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h
index 4379ed512cdf0a..8ea5ed52e28218 100644
--- a/flang/include/flang/Lower/Bridge.h
+++ b/flang/include/flang/Lower/Bridge.h
@@ -14,6 +14,8 @@
#define FORTRAN_LOWER_BRIDGE_H
#include "flang/Common/Fortran.h"
+#include "flang/Frontend/CodeGenOptions.h"
+#include "flang/Frontend/TargetOptions.h"
#include "flang/Lower/AbstractConverter.h"
#include "flang/Lower/EnvironmentDefault.h"
#include "flang/Lower/LoweringOptions.h"
@@ -65,11 +67,13 @@ class LoweringBridge {
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, llvm::StringRef tuneCPU) {
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
+ const Fortran::frontend::CodeGenOptions &codeGenOptions) {
return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
targetCharacteristics, allCooked, triple, kindMap,
loweringOptions, envDefaults, languageFeatures,
- targetMachine, tuneCPU);
+ targetMachine, targetOptions, codeGenOptions);
}
//===--------------------------------------------------------------------===//
@@ -148,7 +152,9 @@ class LoweringBridge {
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU);
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOptions,
+ const Fortran::frontend::CodeGenOptions &codeGenOptions);
LoweringBridge() = delete;
LoweringBridge(const LoweringBridge &) = delete;
diff --git a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
index e45011c8e02a33..2df14f83c11e17 100644
--- a/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
+++ b/flang/include/flang/Optimizer/Dialect/Support/FIRContext.h
@@ -77,6 +77,12 @@ void setIdent(mlir::ModuleOp mod, llvm::StringRef ident);
/// Get the compiler identifier from the Module.
llvm::StringRef getIdent(mlir::ModuleOp mod);
+/// Set the command line used in this invocation.
+void setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine);
+
+/// Get the command line used in this invocation.
+llvm::StringRef getCommandline(mlir::ModuleOp mod);
+
/// Helper for determining the target from the host, etc. Tools may use this
/// function to provide a consistent interpretation of the `--target=<string>`
/// command-line option.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 1d73397d330178..4e8fd44f526a5c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -348,6 +348,12 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
opts.SaveTempsDir = a->getValue();
+ // -record-command-line option.
+ if (const llvm::opt::Arg *a =
+ args.getLastArg(clang::driver::options::OPT_record_command_line)) {
+ opts.RecordCommandLine = a->getValue();
+ }
+
// -mlink-builtin-bitcode
for (auto *a :
args.filtered(clang::driver::options::OPT_mlink_builtin_bitcode))
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 5c86bd947ce73f..909e5909938d87 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -298,7 +298,7 @@ bool CodeGenAction::beginSourceFileAction() {
kindMap, ci.getInvocation().getLoweringOpts(),
ci.getInvocation().getFrontendOpts().envDefaults,
ci.getInvocation().getFrontendOpts().features, targetMachine,
- ci.getInvocation().getTargetOpts().cpuToTuneFor);
+ ci.getInvocation().getTargetOpts(), ci.getInvocation().getCodeGenOpts());
// Fetch module from lb, so we can set
mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ccbb481f472d81..2b42c3e355fbcc 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -6065,7 +6065,9 @@ Fortran::lower::LoweringBridge::LoweringBridge(
const Fortran::lower::LoweringOptions &loweringOptions,
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
const Fortran::common::LanguageFeatureControl &languageFeatures,
- const llvm::TargetMachine &targetMachine, const llvm::StringRef tuneCPU)
+ const llvm::TargetMachine &targetMachine,
+ const Fortran::frontend::TargetOptions &targetOpts,
+ const Fortran::frontend::CodeGenOptions &cgOpts)
: semanticsContext{semanticsContext}, defaultKinds{defaultKinds},
intrinsics{intrinsics}, targetCharacteristics{targetCharacteristics},
cooked{&cooked}, context{context}, kindMap{kindMap},
@@ -6122,11 +6124,13 @@ Fortran::lower::LoweringBridge::LoweringBridge(
fir::setTargetTriple(*module.get(), triple);
fir::setKindMapping(*module.get(), kindMap);
fir::setTargetCPU(*module.get(), targetMachine.getTargetCPU());
- fir::setTuneCPU(*module.get(), tuneCPU);
+ fir::setTuneCPU(*module.get(), targetOpts.cpuToTuneFor);
fir::setTargetFeatures(*module.get(), targetMachine.getTargetFeatureString());
fir::support::setMLIRDataLayout(*module.get(),
targetMachine.createDataLayout());
fir::setIdent(*module.get(), Fortran::common::getFlangFullVersion());
+ if (cgOpts.RecordCommandLine)
+ fir::setCommandline(*module.get(), *cgOpts.RecordCommandLine);
}
void Fortran::lower::genCleanUpInRegionIfAny(
diff --git a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
index 5bd8e2d4336361..01c0be66d1ecc3 100644
--- a/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
+++ b/flang/lib/Optimizer/Dialect/Support/FIRContext.cpp
@@ -130,6 +130,22 @@ llvm::StringRef fir::getIdent(mlir::ModuleOp mod) {
return {};
}
+void fir::setCommandline(mlir::ModuleOp mod, llvm::StringRef cmdLine) {
+ if (cmdLine.empty())
+ return;
+
+ mlir::MLIRContext *ctx = mod.getContext();
+ mod->setAttr(mlir::LLVM::LLVMDialect::getCommandlineAttrName(),
+ mlir::StringAttr::get(ctx, cmdLine));
+}
+
+llvm::StringRef fir::getCommandline(mlir::ModuleOp mod) {
+ if (auto attr = mod->getAttrOfType<mlir::StringAttr>(
+ mlir::LLVM::LLVMDialect::getCommandlineAttrName()))
+ return attr;
+ return {};
+}
+
std::string fir::determineTargetTriple(llvm::StringRef triple) {
// Treat "" or "default" as stand-ins for the default machine.
if (triple.empty() || triple == "default")
diff --git a/flang/test/Driver/frecord-command-line.f90 b/flang/test/Driver/frecord-command-line.f90
new file mode 100644
index 00000000000000..3dc65587ee2563
--- /dev/null
+++ b/flang/test/Driver/frecord-command-line.f90
@@ -0,0 +1,11 @@
+! This only checks that the command line is correctly passed on to the
+! -record-command-line option FC1 option and that the latter does not complain
+! about anything. The correct lowering to a module attribute and beyond will
+! be checked in other tests.
+!
+! RUN: %flang -### -frecord-command-line %s 2>&1 | FileCheck --check-prefix=CHECK-RECORD ...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MLIR part looks good to me, but I cannot speak for clang nor flang 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor questions.
@@ -1980,9 +1980,11 @@ within a command line are combined with spaces; spaces and backslashes within an | |||
argument are escaped with backslashes. This format differs from the format of | |||
the equivalent section produced by GCC with the -frecord-gcc-switches flag. | |||
This option is currently only supported on ELF targets.}]>, | |||
Group<f_clang_Group>; | |||
Group<f_clang_Group>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know whether flang's flow is affected by f_clang_Group
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what you mean by "flang's flow"?
I don't know exactly what f_clang_Group
does. I haven't been able to determine what effect it has on things like help messages. For instance -frecord-command-line
does not appear in either clang --help
, or clang --help-hidden
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kiranchandramohan, gentle ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean't whether f_clang_Group
is processed by flang or not. I see that this is used by some other commands as well. So it is alrite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think f_clang_Group is a legacy group for Clang options not in GCC. f_clang_Group has never gain significant adoption and it's used by any code.
New -f options can just use f_Group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed the group to f_Group
since that does not suggest that the options are exclusive to clang.
79976c8
to
6df2f16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
6df2f16
to
a77d788
Compare
Why do we need the two new files? (added) clang/lib/Driver/ToolChains/CommonUtils.cpp (+76) We have clang/lib/Driver/ToolChains/CommonArgs.cpp and clang/lib/Driver/ToolChain.cpp for shared code. |
Since most of the code in If it is ok to use |
a77d788
to
367519d
Compare
Adding the shared code to
|
Moved code to the |
@MaskRay, If everything looks good to you now, could you approve this? I apologize for being pushy, but this will help us get moving on some other things in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
As you are renaming functions, you can fix the function naming style
tools::function
instead of tools::Function
.
While the style is largely inconsistent, we usually fix the style while renaming functions.
I will do that and merge. Thanks! |
Add support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang.
…the code to CommonArgs instead. That code has been put in the clang::driver namespace.
…uggest that the options are "clang only"
17161f4
to
4e2ceec
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/6766 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/3812 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/3854 Here is the relevant piece of the build log for the reference
|
…02975)" This reverts commit b3533a1. It caused test failures in shared library builds: https://lab.llvm.org/buildbot/#/builders/80/builds/3854
I've had to revert this due to the failures listed above. To reproduce, configure with (the other 2 bots are not called shared libs but they do enable this setting too)
This happens to at least the bbc and tco tools that I notice include a .inc file related to commands. So that might be the issue, but simply removing the include wasn't correct so I leave it to you to figure out. |
Thanks for the instructions on how to reproduce. I'll take a look. |
The underlying issue is due to |
…vm#102975)" This reverts commit b3533a1. It caused test failures in shared library builds: https://lab.llvm.org/buildbot/#/builders/80/builds/3854
…lvm#102975)" The underlying issue was caused by a file included in two different places which resulted in duplicate definition errors when linking individual shared libraries. This was fixed in c3201dd [llvm#109874].
…lvm#102975)" The underlying issue was caused by a file included in two different places which resulted in duplicate definition errors when linking individual shared libraries. This was fixed in c3201dd [llvm#109874].
…lvm#102975)" The underlying issue was caused by a file included in two different places which resulted in duplicate definition errors when linking individual shared libraries. This was fixed in c3201dd [llvm#109874].
Add support for the -frecord-command-line option that will produce the llvm.commandline metadata which will eventually be saved in the object file. This behavior is also supported in clang. Some refactoring of the code in flang to handle these command line options was carried out. The corresponding -grecord-command-line option which saves the command line in the debug information has not yet been enabled for flang.
Notes for reviewers: In #98517, the
tune-cpu
argument was added to the constructor of the lowering bridge object in flang. This PR would have required adding another option to it. It is possible more such arguments may be added in the future. To avoid having to add new arguments each time, theTargetOptions
andCodeGenOptions
objects from the frontend are now passed directly through to the lowering bridge constructor.This required a change to
bbc
where emptyTargetOptions
andCodeGenOptions
objects had to be created.If others have better suggestions for how to handle this, I am happy to make changes.
[EDIT: Fix typo]